Skip to content

Conversation

@Xenius97
Copy link
Contributor

@Xenius97 Xenius97 commented Nov 28, 2025

Closes #2400

This PR adds new getElementsSyncedByPlayer which returns element table synced by player.

Shared:

table getElementsSyncedByPlayer()

tederis
tederis previously approved these changes Dec 2, 2025
Copy link
Member

@tederis tederis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@FileEX FileEX added the enhancement New feature or request label Dec 6, 2025
FileEX
FileEX previously approved these changes Dec 6, 2025
@Xenius97 Xenius97 closed this Jan 6, 2026
@Xenius97 Xenius97 reopened this Jan 6, 2026
Copy link
Member

@qaisjp qaisjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request!

isElementSyncer feedback

  • I have concerns around the argument order of isElementSyncer server-side being confusing. Should the element be first, or the player be first? Currently in your pull request, the player is first.
  • What's the benefit of isElementSyncer server-side when you can already do if element.syncer == player? This is more concise and doesn't introduce confusion around the argument order.

I'd prefer we remove the isElementSyncer portion of this pull request.

getElementsSyncedByPlayer feedback

The filter feels strange to me. Do we have other examples of filters being parsed as comma-separated strings?

The linked issue didn't really explain why they want to filter by type, so for now I'd suggest just returning all the elements. We can always add filtering in a future pull request.

@Xenius97 Xenius97 changed the title Add isElementSyncer for serverside && add new getElementsSyncedByPlayer Add new getElementsSyncedByPlayer Jan 7, 2026
@Xenius97
Copy link
Contributor Author

Xenius97 commented Jan 7, 2026

Thanks for pointing this out!
I’ve removed the isElementSyncer part as suggested, and also removed the filter from getElementsSyncedByPlayer.

@Xenius97 Xenius97 requested a review from qaisjp January 7, 2026 07:07
qaisjp
qaisjp previously approved these changes Jan 7, 2026
Copy link
Member

@qaisjp qaisjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good. Btw it would be good if you merged the CStaticFunctionDefinitions directly into the luadefs.cpp file. (And removed the assertion.)

The CStaticFunctionDefinitions exists mainly for historical reasons and is not necessary for new functions

@Xenius97
Copy link
Contributor Author

Xenius97 commented Jan 7, 2026

The CStaticFunctionDefinitions exists mainly for historical reasons and is not necessary for new functions

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add getPlayerSyncerElements

4 participants